-
Notifications
You must be signed in to change notification settings - Fork 485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rumble cvar and fixes #184
Conversation
Technically, adding the CVar doesn't actually fix #170. The problem shown there with the fishing game is that it specifically checks the pakType to determine if a rumble pack is inserted, and controllers with rumble capabilities aren't recognized as having rumble capabilities because there's no code for modern controllers that specifically sets the pakType as a rumble pack. Then, you'd just need to make u32 func_800AA148(void) { into u32 func_800AA148(void) { for the rumble detection to also appropriately be controlled by the CVar and have the fishing guy correctly tell you rumble is disabled when rumble is manually disabled. |
@jbodner09 This does actually fix the fishing minigame. Because I set the |
I think you're missing to |
Interesting! I added padMgr->padStatus[0].status = 1; to my local build in the same place you have it here, and it didn't change the fishing message, but if multiple people are testing the change as a whole and it works, then who am I to argue? I still think the game should be able to detect the rumble pack on its own without being told to with the CVar though. That way if you have the CVar enabled but no rumblable controller connected, you'll still get the correct fishing message. We could open another issue for that though. |
Reading through this, I think jbodner's method of implementation is way better than a cvar. Would someone actually desire to turn off rumble if it was enabled? Do most games have an option for turning off rumble? |
You need to also change |
Yes, most games do have an option to disable rumble and there is an open issue requesting a feature to toggle it. I could however add different modes for rumble in this PR though. One could be |
Auto and disabled sounds great to me |
only reason i can think of is fisherman text
vs
so if someone wanted to save a few frames but not feel rumble (or test stuff out) it could be useful, but that's an edge case at best auto and disabled seems like the move |
Alright, I have implemented the suggestion. Even if the rumble checkbox is enabled, the game will only think the rumble pak is inserted if the controller also supports rumble. I'm not sure if #183 is out of scope of this PR or not since it seems related to the core game loop so I think it's probably best to see if a maintainer can weigh in on that. If it seems out of scope, then this PR is ready to be undrafted. |
I can see a desire for a slider to set the force amount that the rumble motor uses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably expose the ability to set rumble settings per controller. There can be a new section in the UI for input settings.
In that case, what exactly should be in the per-controller config? Should just the rumble strength be configurable per-controller and have whether or not rumble is enabled be a global setting? (This makes the most sense to me). There is already a rumble intensity slider in the controller menu so I can try and make that affect the per-controller setting for the current controller. |
Makes the most sense to me too. The existing slider goes from 0 to 100%, so if anybody wants to turn off rumble entirely for just a single controller, they can just drop its slider down to 0. |
Ah I forgot we already had a slider. I'd just change the slider to be per controller, but when the controller is at zero strength it is treated as off/disconnected for that controller. I wouldn't be opposed to a global checkmark that disables all four sliders if rumble is off globally. Then internally rumble is treated as disconnected. |
Alright, I went ahead and added controller specific settings for rumble strength (and threw in the gyro info as well for fun). These can be configured from the |
FYI, this is going to conflict hard with #220 if it ends up merging before this does, and if it doesn't, then you'll need to follow up to remove the same kinds of redundant parameters that it does. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge pending conflict resolution.
@Kenix3 Resolved merge conflict. Also removed some unnecessary |
…-overworld-rendering and GI to GID for dungeon rewards
* Rumble indefinitely until turned off * Add rumble cvar * Register CVar * Check if controller can rumble to insert rumble pak * Reduce verbosity of checks * Remove extraneous const_cast * Once again remove extraneous const_cast * Add per-controller settings * Add nice spacing * Only display controller entry if pad connected * Const some stuff
* Rumble indefinitely until turned off * Add rumble cvar * Register CVar * Check if controller can rumble to insert rumble pak * Reduce verbosity of checks * Remove extraneous const_cast * Once again remove extraneous const_cast * Add per-controller settings * Add nice spacing * Only display controller entry if pad connected * Const some stuff
This pull request closes #182 and closes #170 by adding a rumble cvar. While the cvar is enabled, the game will think the rumble pak is inserted and SoH will vibrate the controller normally. When disabled, the game will think there is no rumble pak and SoH will not rumble the controller.
This PR also fixes another problem by rumbling the controller indefinitely until the boolean is set to false instead of doing 1ms rumbles like the previous implementation. This fixes the issue of rumble not staying on consistently during vibration. Accordingly, this PR stops rumble when SoH exits.
The reason I drafted this PR is because I want to also address #183 and get it into the PR but currently I am not sure how to fix it.